mPUSHn()/mPUSHi()/mPUSHu() convert from slow sv_set() to newSV()+2mor…#23148
mPUSHn()/mPUSHi()/mPUSHu() convert from slow sv_set() to newSV()+2mor…#23148bulk88 wants to merge 1 commit intoPerl:bleadfrom
Conversation
…tal() commit d82b684 - Apr 30 2004 - Steve Hay Document limitations in PUSHi et al macros and add new mPUSHi et al macros which is day 1 of these macros, introduced this not ideal pattern doing "sv_setiv(PUSHmortal,)". sv_set*() funcs are much more complex/more work at runtime, since they must undo *everything* prior in the SV* and maybe upgrade the body and copy over fields from the prev "body[less]". newSV*() funcs dont have to do that. sv_2mortal() is very small and light weight internally, so rmving sv_newmortal() then adding sv_2mortal() isn't a problem, but for full disclosure, this commit makes better macros mPUSHn()/mPUSHi()/mPUSHu() than before, but this commit isn't perfect. sv_2mortal() is public api, and while it has exactly 1 conditional fn call in it (stack grower), it is a little bit heavier and does more work than a perfection hand written solution (see PUSH_EXTEND_MORTAL__SV_C). sv_2mortal() contains 4 branches, PUSH_EXTEND_MORTAL__SV_C has 1 branch. Another benefit of this commit is XS code that looks like if(r=c_syscall()) PUSHs(sv_2mortal(newSViv(r))); is better than if(r=c_syscall()) sv = newSV(); PUSHs(sv); sv_setiv(r); b/c var r, will effortlessly flow from the volatile retval register to a volatile outgoing "C stk" register (xfering control to newSViv()) and doesn't need to be save/restored from a non-vol storage location to preserve var r around a extern linked C fn. The 2004 commit above also has a same title medium sized P5P thread. This 2004 patch doesn't have a spotless history as a patch, and it went through multiple revisions b/c of typos and mistakes at the time, until it was decided to publish it. That commit probably did the "sv_setiv(PUSHmortal,)" pattern b/c of copy pasting from the existing TARG macros. And the commit is an improvement/fix for flaws or unfriendlyness that TARG macros had.
|
bump, randomly ran into this annoyance again fixing a random cpan xs mod, Id rather see this centrally fixed in blead perl headers than me overriding stock perls by hand as a CPAN XS patch author And for next Christmas, for Santa to add this bug fix to Devel::PPPort, back porting the bug fix to all Perl VM binaries in the world. |
|
I object to pushing this p.r. until the commit message reads sensibly. |
Yeah that is a recurring challenge. Let's try to find a path out of this situation. Your commit message reads as a stream-of-thought. In my experience, writing any good documentation starts with the asking questions that the reader is expected to have. For the body of a commit message that could be for example
Something like this perhaps:
|
3 participants
…tal()
commit d82b684 - Apr 30 2004 - Steve Hay
Document limitations in PUSHi et al macros and add new mPUSHi et al macros
which is day 1 of these macros, introduced this not ideal pattern doing "sv_setiv(PUSHmortal,)". sv_set*() funcs are much more complex/more work at runtime, since they must undo everything prior in the SV* and maybe upgrade the body and copy over fields from the prev "body[less]". newSV*() funcs dont have to do that. sv_2mortal() is very small and light weight internally, so rmving sv_newmortal() then adding sv_2mortal() isn't a problem, but for full disclosure, this commit makes better macros mPUSHn()/mPUSHi()/mPUSHu() than before, but this commit isn't perfect. sv_2mortal() is public api, and while it has exactly 1 conditional fn call in it (stack grower), it is a little bit heavier and does more work than a perfection hand written solution (see PUSH_EXTEND_MORTAL__SV_C). sv_2mortal() contains 4 branches, PUSH_EXTEND_MORTAL__SV_C has 1 branch.
Another benefit of this commit is XS code that looks like if(r=c_syscall()) PUSHs(sv_2mortal(newSViv(r)));
is better than
if(r=c_syscall()) sv = newSV(); PUSHs(sv); sv_setiv(r); b/c var r, will effortlessly flow from the volatile retval register to a volatile outgoing "C stk" register (xfering control to newSViv()) and doesn't need to be save/restored from a non-vol storage location to preserve var r around a extern linked C fn.
The 2004 commit above also has a same title medium sized P5P thread. This 2004 patch doesn't have a spotless history as a patch, and it went through multiple revisions b/c of typos and mistakes at the time, until it was decided to publish it. That commit probably did the "sv_setiv(PUSHmortal,)" pattern b/c of copy pasting from the existing TARG macros. And the commit is an improvement/fix for flaws or unfriendlyness that TARG macros had.
too lazy to edit Devel::PPPort, but everyone at P5P neglects that area of core